Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

serde: give access to underlying writer #15

Closed
wants to merge 1 commit into from

Conversation

vmx
Copy link
Contributor

@vmx vmx commented Apr 12, 2022

When using the Serializer from an external crate it can be useful
to be able to access the underlying writer.

It turns out that the code I posted at #13 (comment) only works with this patch.

When using the `Serializer` from an external crate it can be useful
to be able to access the underlying writer.
@@ -16,6 +16,10 @@ impl<W> Serializer<W> {
pub fn into_inner(self) -> W {
self.writer
}

pub fn writer(&mut self) -> &mut W {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like it to be called get_mut by convention.

@vmx
Copy link
Contributor Author

vmx commented Apr 12, 2022

In turns out that I probably don't need this change. I tried to wrap your Serializer in my own custom serializer and changed just a few serialize_*() functions. I had hoped I could re-use the serialize implementations for sequences, maps, tuples and structs. But as they call into their serializer (via self.ser), they won't call into my own serializer. This means that I would need to implement those myself and also need code to call them. There won't be much left to wrap, so I think it's easier to just copy and paste the current serializer, adapt it to my needs and disable the serde1 feature, so that I only rely on core.

If you still want this change, I'll rename the function to get_mut(), if not, feel free to close this PR.

PS: Thanks for always responding that fast!

@quininer
Copy link
Owner

If no one needs it, I tend to keep it private. Thank you.

@quininer quininer closed this Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants